New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix timing out requests too early #2606
Conversation
Fixes puma#2574 (regression in v5.0.3). Co-authored-by: Daniel Huckstep <daniel@huckstep.ca> Co-authored-by: Will Jordan <will@code.org>
LGTM. Maybe change def test_timeout_in_data_phase
@server.first_data_timeout = 1
server_run
sock = send_http "POST / HTTP/1.1\r\nHost: test.com\r\nContent-Type: text/plain\r\nContent-Length: 5\r\n\r\n"
sleep 1.15
sock << "Hello"
data = sock.gets
assert_equal "HTTP/1.1 408 Request Timeout\r\n", data
end |
So the 'Windows' guy (me) now does 95% of his Puma work with WSL2/Ubuntu20.04. Sorry for that. Not sure what I think of the |
I saw you setting the timing to zero, I'm running the following in my fork right now... def test_timeout_in_data_phase
@server.first_data_timeout = 1
server_run
sock = send_http "POST / HTTP/1.1\r\nHost: test.com\r\nContent-Type: text/plain\r\nContent-Length: 5\r\n\r\n"
sleep 1.15
sock << "Hello"
if Puma::IS_WINDOWS
assert_raises(Errno::ECONNABORTED) { sock.gets }
else
data = sock.gets
assert_equal "HTTP/1.1 408 Request Timeout\r\n", data
end
end |
Complete request to ensure short timeout is used properly.
Looks like both versions are passing. I also had a nonrelated failure... |
I made the change with |
It looks like a Windows |
Works for me. |
* Fix timing out when we get a slow request Fixes puma#2574 (regression in v5.0.3). Co-authored-by: Daniel Huckstep <daniel@huckstep.ca> Co-authored-by: Will Jordan <will@code.org> * Update test_timeout_in_data_phase Complete request to ensure short timeout is used properly. Co-authored-by: Daniel Huckstep <daniel@huckstep.ca>
Description
Fixes #2574 (a regression released in v5.0.3), with an added test to prevent future regressions.
See also
#2575 extends this fix by adding a
between_bytes_timeout
configuration option that allows for different timeout values for the initial data received by the client, and subsequent chunks of data. This extra option may eventually be useful but this PR is intended to more narrowly fix the regression first.Your checklist for this pull request
[ci skip]
to the title of the PR.#issue
" to the PR description or my commit messages.